BLUE-280: Add Publisher for Transaction Digest to IPFS via Web3.Storage #86
Conversation
PR Reviewer Guide 🔍
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/txDigester/ipfsPublisher.ts
Outdated
| try { | ||
| const plan = await account.plan.get() | ||
| if (!plan.ok) { | ||
| console.error(`❌ ${admin_email} does not have an active plan subscribed on Web3.Storage.`) |
There was a problem hiding this comment.
we should not log email id. let's mask it using industry standard logic or remove it.
There was a problem hiding this comment.
Yeah we can mask it for sure, although it can't be exploited, I consoled it initially just for testing purpose.
src/txDigester/ipfsPublisher.ts
Outdated
| isPublisherActive = false | ||
| } catch (error) { | ||
| isPublisherActive = false | ||
| console.error(`❌ Error while Uploading Digest (w/ Hash: ${data.hash}) to IPFS: `) |
There was a problem hiding this comment.
I included the hash only to keep the log line short and the hash makes it easier to search in a DB (if someone wants to quickly look it up since its just 1 value).
I'll add in start and end cycles too.
achal-singh
force-pushed
the
BLUE-280
branch
from
c691bb5
to
eb61cc7
Compare
| isPublisherActive = true | ||
| console.log(`Uploading TX Digest for Cycle Range ${data.cycleStart} to ${data.cycleEnd}`) | ||
| await client.setCurrentSpace(rootDID as Web3StorageRootDID) | ||
| console.log(`Uploading Data to Root-DID: ${rootDID}`) |
Check warning
Code scanning / CodeQL
Log injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the log injection issue, we need to sanitize the rootDID value before logging it. Specifically, we should remove any newline characters from the rootDID to prevent log forgery. This can be done using String.prototype.replace to ensure no line endings are present in the user input.
| @@ -39,3 +39,4 @@ | ||
| await client.setCurrentSpace(rootDID as Web3StorageRootDID) | ||
| console.log(`Uploading Data to Root-DID: ${rootDID}`) | ||
| const sanitizedRootDID = rootDID.replace(/\n|\r/g, "") | ||
| console.log(`Uploading Data to Root-DID: ${sanitizedRootDID}`) | ||
|
|
| try { | ||
| isPublisherActive = true | ||
| console.log('Initialising IPFS publisher...') | ||
| if (!adminEmail || !rootDID) { |
Check failure
Code scanning / CodeQL
User-controlled bypass of security check High
| try { | ||
| isPublisherActive = true | ||
| console.log('Initialising IPFS publisher...') | ||
| if (!adminEmail || !rootDID) { |
Check failure
Code scanning / CodeQL
User-controlled bypass of security check High
achal-singh
force-pushed
the
BLUE-280
branch
3 times, most recently
from
f6fc2c2
to
f56041b
Compare
achal-singh
force-pushed
the
BLUE-280
branch
from
f56041b
to
0d28777
Compare
| } | ||
| client = await W3SClient.create() | ||
|
|
||
| console.log(`Logging into Web3.Storage with Account: ${maskedEmail(adminEmail)}`) |
Check warning
Code scanning / CodeQL
Log injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the log injection issue, we need to sanitize the adminEmail before logging it. Specifically, we should remove any newline characters from the adminEmail to prevent log injection attacks. This can be done using String.prototype.replace to remove newline characters.
| @@ -78,2 +78,6 @@ | ||
|
|
||
| const sanitizeEmail = (email: string): string => { | ||
| return email.replace(/\n|\r/g, ""); | ||
| } | ||
|
|
||
| const maskedEmail = (email: string): string => { | ||
| @@ -94,3 +98,3 @@ | ||
|
|
||
| console.log(`Logging into Web3.Storage with Account: ${maskedEmail(adminEmail)}`) | ||
| console.log(`Logging into Web3.Storage with Account: ${maskedEmail(sanitizeEmail(adminEmail))}`) | ||
| if (!isAuthWithEmail(adminEmail as Web3StorageAdminEmail)) { |
|
|
||
| console.log(`Logging into Web3.Storage with Account: ${maskedEmail(adminEmail)}`) | ||
| if (!isAuthWithEmail(adminEmail as Web3StorageAdminEmail)) { | ||
| console.log(`⏳ Owner of ${adminEmail} needs to approve the Web3.Storage Auth request to proceed.`) |
Check warning
Code scanning / CodeQL
Log injection Medium
| } | ||
|
|
||
| await client.setCurrentSpace(rootDID as Web3StorageRootDID) | ||
| console.log(`Current Space set to DID: ${rootDID}`) |
Check warning
Code scanning / CodeQL
Log injection Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI about 1 month ago
To fix the log injection issue, we need to sanitize the rootDID value before logging it. Specifically, we should remove any newline characters from the rootDID to prevent log injection. This can be done using the String.prototype.replace method to replace newline characters with an empty string.
| @@ -102,3 +102,4 @@ | ||
| await client.setCurrentSpace(rootDID as Web3StorageRootDID) | ||
| console.log(`Current Space set to DID: ${rootDID}`) | ||
| const sanitizedRootDID = rootDID.replace(/\n|\r/g, "") | ||
| console.log(`Current Space set to DID: ${sanitizedRootDID}`) | ||
| isPublisherActive = false |
Summary
ipfsPublisher(as a part of the Transaction DIgest Cron Server) to upload/publish the same Transaction Digest to the IPFS Network via the Web3.Storage provider.Linear Task
Merging Notes
It'd be good to merge BLUE-283's PR into BLUE-280 PR first before merging
BLUE-280todev.Testing Notes
Space, set any name as per your preference. At the top of the screen you'll see a Root DID string (starts withdid:key:...) of your Space.root_didandadmin_emailfields in the archiver-config.json file respectively and set theenableSavingToWeb3Storageflag totrue.npm run txDigestCronServerORpm2 start --name txDigestCron npm -- run txDigestCronServerto launch thetxDigestCronServerprocess.